http tap: add request/response trailer matching#5804
Conversation
1) Add request/response trailer matching 2) Output request/response trailers 3) Refactor matchers to reduce boilerplate and make it harder to make mistakes when adding new update functions. 4) Split out a match configuration for HTTP request and response into individual things like request headers, request trailers, etc. This will make it easier and more logical to add various types of body matching and wire it up using the existing and/or logic. Signed-off-by: Matt Klein <mklein@lyft.com>
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
LGTM (assuming you have full test coverage with the new test). One API-level minor comment.
Signed-off-by: Matt Klein <mklein@lyft.com>
|
@htuch updated |
| // HTTP request match configuration. | ||
| HttpRequestMatch http_request_match = 5; | ||
| // HTTP request headers match configuration. | ||
| HttpHeadersMatch http_request_headers_match = 5; |
There was a problem hiding this comment.
should we organize this a bit, just like how BufferedTrace organized? i.e.
message HttpMessageMatch {
HttpHeadersMatch headers_match;
HttpTrailersMatch trailers_match;
}
?
There was a problem hiding this comment.
This is how I had this originally, but IMO we don't want this, because it makes it much more confusing on how to apply and/or logic.
lizan
left a comment
lizan
left a comment
There was a problem hiding this comment.
LGTM,
One thing is probably not actionable in this PR but just want to note:
gRPC HTTP2 protocol maps a response with one HEADERS w/END_STREAM as Trailers-Only response. So it means this matching frame (as well as access logger matchers, which is what #5682 addresses) isn't easy to make it match all grpc-status/grpc-message.
Yes, agreed. FWIW my plan, once I get the basics implemented, is to actually have more interesting matchers like JSON, gRPC, etc. |
1) Add request/response trailer matching 2) Output request/response trailers 3) Refactor matchers to reduce boilerplate and make it harder to make mistakes when adding new update functions. 4) Split out a match configuration for HTTP request and response into individual things like request headers, request trailers, etc. This will make it easier and more logical to add various types of body matching and wire it up using the existing and/or logic. Signed-off-by: Matt Klein <mklein@lyft.com> Signed-off-by: Fred Douglas <fredlas@google.com>
3 participants
to make mistakes when adding new update functions.
into individual things like request headers, request trailers,
etc. This will make it easier and more logical to add various
types of body matching and wire it up using the existing and/or
logic.
Risk Level: Low
Testing: New tests
Docs Changes: Complete
Release Notes: N/A